Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor getter caching based on keypath state #223

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

jordangarcia
Copy link
Contributor

@jordangarcia jordangarcia commented Sep 27, 2016

The current version of NuclearJS uses a cache key consisting of store
states (monotomically incresing ID per store). This has the
disadvantage of allowing only a single level of depth when figuring out
if a cache entry is stale. This leads to poor performance when the
shape of a Reactor's state is more deep than wide, ie a store having
multiple responsibilities for state tracking.

The implementation is as follows:

  • Consumer can set the maxCacheDepth when instantiating a reactor
  • Getters are broken down into the canonical set of keypaths based on
    the maxCacheDepth
  • Add a keypath tracker abstraction to maintain the state value of all
    tracked keypaths
  • After any state change (Reactor.__notify) dirty keypaths are
    resolved and then based on which keypaths have changed any dependent
    observers are called

@loganlinn

The current version of NuclearJS uses a cache key consisting of store
states (monotomically incresing ID per store).  This has the
disadvantage of allowing only a single level of depth when figuring out
if a cache entry is stale.  This leads to poor performance when the
shape of a Reactor's state is more deep than wide, ie a store having
multiple responsibilities for state tracking.

The implementation is as follows:

- Consumer can set the maxCacheDepth when instantiating a reactor
- Getters are broken down into the canonical set of keypaths based on
the maxCacheDepth
- Add a keypath tracker abstraction to maintain the state value of all
tracked keypaths
- After any state change (`Reactor.__notify`) dirty keypaths are
resolved and then based on which keypaths have changed any dependent
observers are called
Instead of doing a full recursive traversal of the keypathStates tree,
keep track of all changed paths as use that as an indicator of the paths
the need to be cleaned
@mjc1283 mjc1283 requested a review from loganlinn June 19, 2017 22:19
@alex-kovoy
Copy link

Just curious if this is going to be merged any time soon?

@loganlinn loganlinn removed their request for review October 7, 2020 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants